Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Internationalize scenarios #676

Merged
merged 6 commits into from
Feb 16, 2017
Merged

Internationalize scenarios #676

merged 6 commits into from
Feb 16, 2017

Conversation

fpagnoux
Copy link
Member

@fpagnoux fpagnoux commented Feb 2, 2017


return json_or_python_to_test_case

def suggest(self):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cbenz @benjello
Does anyone know why we need this, and why it was separated from the rest of the test_case processing ?

It is called when using the api, when running tests, etc. Should it just be part of the normal test_case processing workflow ? Are there cases where you don't want to call this method ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it was used for th UI. @cbenz should know better

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, suggest was introduced for the UI and could be removed (this would imply to remove a flag in the API code).

why it was separated from the rest of the test_case processing ?

For code clarity I think.

@fpagnoux fpagnoux requested review from cbenz and benjello February 2, 2017 00:32
Copy link
Member

@benjello benjello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not an expert on this part but it seems ok for me

Copy link
Member

@cbenz cbenz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you moved some code around, I don't see any new design here and it's OK for me.


return json_or_python_to_test_case

def suggest(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, suggest was introduced for the UI and could be removed (this would imply to remove a flag in the API code).

why it was separated from the rest of the test_case processing ?

For code clarity I think.

@fpagnoux fpagnoux self-assigned this Feb 16, 2017
@cbenz cbenz force-pushed the internationalize-scenarios branch from a687b95 to 6961fc3 Compare February 16, 2017 18:45
@cbenz cbenz force-pushed the internationalize-scenarios branch from 6961fc3 to 4900ce9 Compare February 16, 2017 18:46
CHANGELOG.md Outdated
@@ -1,6 +1,10 @@
# Changelog


## 14.0.0
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je ne pense pas que ce soit un breaking change, si ?
A priori c'est même complètement transparent pour l'utilisateur.

@fpagnoux fpagnoux force-pushed the internationalize-scenarios branch from 4f56d53 to 2707ad8 Compare February 16, 2017 19:48
@fpagnoux fpagnoux merged commit 90f628a into master Feb 16, 2017
@fpagnoux fpagnoux deleted the internationalize-scenarios branch February 16, 2017 20:11
@cbenz
Copy link
Member

cbenz commented Feb 17, 2017

Je ne pense pas que ce soit un breaking change, si ?
A priori c'est même complètement transparent pour l'utilisateur.

Tu as raison, c'est une erreur. Merci d'avoir modifié.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants